Fix libxml failure - #39
Conversation
b093d3b to
199f0fd
Compare
There was a problem hiding this comment.
Pull request overview
This PR attempts to fix libxml failures on Windows by bundling a zlib1.dll binary and adding platform-specific dependency handling. The changes include adding a binary DLL file, a debug script, CI workflow modifications, and Gemfile updates for Windows-specific libxml-ruby version pinning.
Key Changes
- Added zlib1.dll binary to dlls/ directory for Windows libxml support
- Created debug.rb script to diagnose and patch PATH for DLL loading
- Pinned libxml-ruby to version 5.0.4 specifically for Windows platform
- Added rubocop-related gems and CI debug step
Reviewed changes
Copilot reviewed 3 out of 6 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| dlls/zlib1.dll | Binary DLL file added to provide zlib support for Windows |
| debug.rb | Debug script to locate gems and modify PATH for DLL discovery |
| Gemfile | Platform-specific libxml-ruby versioning and additional rubocop gems |
| .github/workflows/rake.yml | Added CI step to run debug script after Ruby setup |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # all_files = Dir.glob("#{spec.full_gem_path}/**/*").select { |f| File.file?(f) } | ||
|
|
||
| # all_files.each do |file| | ||
| # puts file | ||
| # end | ||
|
|
||
| # check PATH | ||
| puts ENV.fetch("PATH", nil) | ||
|
|
||
| # patch lib/libxml-ruby.rb | ||
| # filepath = "a.txt" | ||
| filepath = File.expand_path("#{spec.full_gem_path}/dlls") | ||
| ENV["PATH"] = "#{ENV.fetch("PATH", nil)};#{filepath}" | ||
|
|
||
| puts "Patched PATH:" | ||
| puts ENV.fetch("PATH", nil) | ||
|
|
||
| # original_content = File.read(filepath) | ||
|
|
||
| # line_to_add = "ENV['PATH'] = ENV['PATH'] + ';' + File.expand_path(File.dirname(__FILE__))\nputs 'Patched PATH: ' + ENV['PATH']" | ||
|
|
||
| # # Combine the new line with the original content | ||
| # new_content = line_to_add + "\n" + original_content | ||
|
|
||
| # # Overwrite the file with the new content | ||
| # File.write(filepath, new_content) | ||
|
|
||
| # puts "new_content================" | ||
| # puts new_content | ||
| # puts "===========================" | ||
|
|
There was a problem hiding this comment.
This debug script contains extensive commented-out code (lines 8-12, 17-37) that should be removed before merging. Commented code clutters the codebase and can be retrieved from version control history if needed.
| # all_files = Dir.glob("#{spec.full_gem_path}/**/*").select { |f| File.file?(f) } | |
| # all_files.each do |file| | |
| # puts file | |
| # end | |
| # check PATH | |
| puts ENV.fetch("PATH", nil) | |
| # patch lib/libxml-ruby.rb | |
| # filepath = "a.txt" | |
| filepath = File.expand_path("#{spec.full_gem_path}/dlls") | |
| ENV["PATH"] = "#{ENV.fetch("PATH", nil)};#{filepath}" | |
| puts "Patched PATH:" | |
| puts ENV.fetch("PATH", nil) | |
| # original_content = File.read(filepath) | |
| # line_to_add = "ENV['PATH'] = ENV['PATH'] + ';' + File.expand_path(File.dirname(__FILE__))\nputs 'Patched PATH: ' + ENV['PATH']" | |
| # # Combine the new line with the original content | |
| # new_content = line_to_add + "\n" + original_content | |
| # # Overwrite the file with the new content | |
| # File.write(filepath, new_content) | |
| # puts "new_content================" | |
| # puts new_content | |
| # puts "===========================" | |
| # check PATH | |
| puts ENV.fetch("PATH", nil) | |
| filepath = File.expand_path("#{spec.full_gem_path}/dlls") | |
| ENV["PATH"] = "#{ENV.fetch("PATH", nil)};#{filepath}" | |
| puts "Patched PATH:" | |
| puts ENV.fetch("PATH", nil) |
|
|
||
| require "rubygems" | ||
| # spec = Gem::Specification.find_by_name("libxml-ruby") | ||
| spec = Gem::Specification.find_by_name("moxml") |
There was a problem hiding this comment.
The hardcoded gem name "moxml" appears to be incorrect or test-specific. If this is a debug script intended to remain in the repository, it should use a configurable or correct gem name. If this is only for temporary debugging, it should not be committed.
| spec = Gem::Specification.find_by_name("moxml") | |
| gem_name = ENV["GEM_NAME"] || ARGV[0] || "libxml-ruby" | |
| spec = Gem::Specification.find_by_name(gem_name) |
| echo "---" > .bundle/config 2>&1 || true | ||
| echo 'BUNDLE_BUILD__LIBXML___RUBY: "--with-xml2-dir=C:/vcpkg/installed/x64-windows --with-xml2-include=C:/vcpkg/installed/x64-windows/include/libxml2 --with-xml2-lib=C:/vcpkg/installed/x64-windows/lib"' >> .bundle/config 2>&1 || true | ||
| after-setup-ruby: | | ||
| bundle exec ruby debug.rb 2>&1 || true |
There was a problem hiding this comment.
The after-setup-ruby step runs the debug script but always succeeds due to || true. This masks failures and defeats the purpose of running it in CI. Either:
- Remove
|| trueto catch failures, or - Remove this debug step entirely if it's not needed for CI validation
| bundle exec ruby debug.rb 2>&1 || true | |
| bundle exec ruby debug.rb 2>&1 |
| if Gem.win_platform? | ||
| gem "libxml-ruby", "5.0.4" | ||
| else | ||
| gem "libxml-ruby" | ||
| end |
There was a problem hiding this comment.
The libxml-ruby version is pinned to exactly "5.0.4" on Windows but allows any version on other platforms. This inconsistency could lead to different behavior across platforms. Consider:
- Documenting why Windows requires this specific version
- Using the same version constraint across platforms if possible
- Adding a comment explaining the platform-specific versioning
| if Gem.win_platform? | |
| gem "libxml-ruby", "5.0.4" | |
| else | |
| gem "libxml-ruby" | |
| end | |
| # Pinning libxml-ruby to 5.0.4 for all platforms to ensure consistent behavior. | |
| # If a newer version is required, update this constraint after verifying cross-platform compatibility. | |
| gem "libxml-ruby", "5.0.4" |
0687768 to
cfb2b91
Compare
051c767 to
fe79ac8
Compare
Adds a dedicated Windows workflow that installs libxml2, ragel, and the UCRT64 toolchain from MSYS2, configures libxml-ruby's extension build against them, prepares the vendored oga/ruby-ll forks, and runs the adapter spec suite (which includes the libxml adapter contract). Replaces the exploratory state of this branch: the commented-out pull_request trigger, debug.rb workflow step, and vcpkg-based before-setup-ruby on the metanorma/ci generic workflow are all dropped in favor of this self-contained workflow matching the structure of rake.yml.
fe79ac8 to
4d7fb10
Compare
stackprof's native extension uses SIGPROF/sigaction, which do not exist on Windows, so bundle install fails there.
No description provided.